-
Notifications
You must be signed in to change notification settings - Fork 132
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Groups: add minimal_generating_set #2816
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2816 +/- ##
==========================================
- Coverage 80.68% 80.68% -0.01%
==========================================
Files 456 456
Lines 64755 64762 +7
==========================================
+ Hits 52250 52251 +1
- Misses 12505 12511 +6
|
e4015b5
to
e7471cb
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks.
The definition of rank
does not agree with rank
for GrpAbFinGen
.
Perhaps the definition of is_free
should be more explicit.
(See my comments.)
src/Groups/group_constructors.jl
Outdated
end | ||
GAP.Globals.SetRankOfFreeGroup(G.X, n) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see: RankOfFreeGroup
belongs to GAP's FGA package, therefore the function FreeGroup
cannot set this attribute. Thus the situation here is better.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Aye.
Regarding the definition of rank(::GrpAbFinGen)
differing (it returns the Z-rank, or the "Hirsch length" to use polycyclic group terminology): ahh, that's a shame. Dang. Well, I could rename this function _rank
for now, so that I can use it in the printing code (that was what motivated this PR), and then possibly again restrict it to support "free fp groups" only. And then worry about how to expose this to users later.
src/Groups/GAPGroups.jl
Outdated
@doc raw""" | ||
is_free(G::GAPGroup) | ||
|
||
Return whether `G` is a free group. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should say what is meant with "is a free group".
GAP's IsFreeGroup
means essentially a group created with free_group
or a subgroup of such a group.
The GAP documentation is also vague about this, but it states that groups consisting of elements in the filter IsAssocWordWithInverse
are regarded as free groups.
One could have the idea that "free group" means a group that is free on some set of elements, and then each trivial group is free on the empty set, and certain factors of free groups (for example where just some generators are mapped to the identity) are also free.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for this important observation.
Adding to it, also any infinite cyclic group (e.g. represented as a pcp group) would be "free". And yeah, in GAP we have this mathematically surprising situation then:
gap> G:=AbelianGroup([0]);
<fp group of size infinity on the generators [ f1 ]>
gap> IsFreeGroup(G);
false
gap> IsFreeGroup(FreeGroup(1));
true
So this is a case were one needs to distinguish between "mathematically free" (on some unknown set) and "technically free" (created as a free group), for lack of better terminology.
Both are of independent interest.... But I think a function is_free
for groups should aim to check for the mathematical property, though of course it cannot in general (as long as we can't generally decide whether a fp group is trivial, we also can't decide freeness). But we can do it in many other cases, e.g. for finite groups (where is_free(G) = is_trivial(G)
), pcp groups (either trivial or infinite cyclic, both can be tested for efficiently). For fp groups, we could make a best effort: compute abelian invariants; if any are non-zero, return false
; otherwise we can read of the potential rank of the "free" group (if it is free); then use TzGoGo
to try and simply (a copy of) the presentation to see if it is "obviously" free (= no relations left).
But of course what I really am using is the "technical interpretation". I actually thought already that perhaps we should split the FPGroup
type not just into FPGroup
and SubFPGroup
(as we already discussed at PR #2672), but in fact also have FreeGroup
(and perhaps also SubFreeGroup
). In that case the "technical" check becomes a type check.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I think that the "technical interpretation" is used more frequently; also GAP's IsFreeGroup
is just a filter, there was no aim to support a check for the mathematical property.
So do we need a better name for the "technical interpretation", in order to keep is_free
for the mathematical one,
or do we decide that we introduce the type FreeGroup
as mentioned above?
(I guess the name IsFreeGroup
was chosen because people did not think that checking the mathematical property for arbitrary f. p. groups would be interesting. Moreover, as soon as such a function would be available, somebody would come and ask for a function that checks whether a group is free on a given set, and for a function that returns a set on which a given group is free ...)
src/Groups/GAPGroups.jl
Outdated
""" | ||
rank(G::GAPGroup) | ||
|
||
Return the rank of the group `G`, i.e., the minimal size of a generating set. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
GrpAbFinGen
defines rank
as the rank of the free part. Thus torsion groups created with abelian_group
have always rank zero. If we really want to define rank
for GAPGroup
as the cardinality of a minimal generating set then perhaps we should mention this.
e7471cb
to
fbc0e3f
Compare
Also add disabled `rank` and `is_free` methods for groups, to be enabled (or removed) in future updates
fbc0e3f
to
be196ad
Compare
In order to move forward, I have for now disabled the |
Co-authored-by: Johannes Schmitt <[email protected]>
No description provided.